Skip to content

Conversation

@azoitl
Copy link
Contributor

@azoitl azoitl commented Dec 15, 2024

The eclipse.appName sytem property allows users to specify on the command line the appName to be used for a specific Eclipse instance. The appName Display property is used by window managers and desktop environments for identification and grouping. This is helpful to distinguish Eclipse instances (e.g., JDT vs CDT) as per default these all are using the same identifier.

eclipse-packaging/packages#253

The eclipse.appName sytem property allows users to specify on the
command line the appName to be used for a specific Eclipse instance. The
appName Display property is used by window managers and
desktop environments for identification and grouping. This is helpful to
distinguish Eclipse instances (e.g., JDT vs CDT) as per default these
all are using the same identifier.

eclipse-packaging/packages#253
@github-actions
Copy link
Contributor

Test Results

 1 213 files   -  1   1 213 suites   - 1   1h 4m 41s ⏱️ + 13m 2s
 7 729 tests ± 0   7 490 ✅  - 11  238 💤 +10  1 ❌ +1 
16 187 runs   - 45  15 676 ✅  - 88  510 💤 +42  1 ❌ +1 

For more details on these failures, see this check.

Results for commit e0a9c39. ± Comparison against base commit 5334e3c.

This pull request skips 10 tests.
org.eclipse.ui.genericeditor.tests.CompletionTest ‑ testCompletion
org.eclipse.ui.genericeditor.tests.CompletionTest ‑ testCompletionFreeze_bug521484
org.eclipse.ui.genericeditor.tests.CompletionTest ‑ testCompletionService
org.eclipse.ui.genericeditor.tests.CompletionTest ‑ testCompletionUsingViewerSelection
org.eclipse.ui.genericeditor.tests.CompletionTest ‑ testMoveCaretBackUsesAllProcessors_bug522255
org.eclipse.ui.genericeditor.tests.HoverTest ‑ testEnabledWhenHover
org.eclipse.ui.genericeditor.tests.HoverTest ‑ testMultipleHover
org.eclipse.ui.genericeditor.tests.HoverTest ‑ testProblemHover
org.eclipse.ui.genericeditor.tests.HoverTest ‑ testSingleHover
org.eclipse.ui.genericeditor.tests.ShowInformationTest ‑ testInformationControl

azoitl added a commit to azoitl/eclipse.platform.releng.aggregator that referenced this pull request Dec 16, 2024
The eclipse.appName sytem property allows users to specify on the
command line the appName to be used for a specific Eclipse instance. The
appName Display property is used by window managers and
desktop environments for identification and grouping. This is helpful to
distinguish Eclipse instances (e.g., JDT vs CDT) as per default these
all are using the same identifier.

Eclipse UI PR:
eclipse-platform/eclipse.platform.ui#2631
Root Issue: eclipse-packaging/packages#253
@azoitl
Copy link
Contributor Author

azoitl commented Dec 16, 2024

Updated the documentation with eclipse-platform/eclipse.platform.releng.aggregator#2663

I only changed run-time options as for me it didn't fit in the running eclipse file.

Copy link
Contributor

@jonahgraham jonahgraham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM - I certainly approve the concept and think this should go in now for M1. But I don't know whether this meets expectations for this code.

I also think this should be added to N&N for platform so that articles such as Gayan's https://medium.com/@gayanper/installing-multiple-eclipse-in-gnome-29db46f57401 can be updated to not require editing XML files in the install anymore.

The N&N source is https://github.com/eclipse-platform/www.eclipse.org-eclipse/blob/master/news/4.35/platform.html which renders to https://eclipse.dev/eclipse/news/4.35/platform.php

@merks merks merged commit d2f7367 into eclipse-platform:master Dec 16, 2024
14 of 17 checks passed
@merks
Copy link
Contributor

merks commented Dec 16, 2024

Yes, it's simple and effective. Thanks!

@azoitl
Copy link
Contributor Author

azoitl commented Dec 16, 2024

Many thanks to @merks and @jonahgraham for helping me find the right solution and place to fix it.

N&N PR will come either later today or tomorrow.

@azoitl azoitl deleted the classParamHandling branch December 16, 2024 15:55
@vogella
Copy link
Contributor

vogella commented Dec 16, 2024

@merks in case the reason for a build failure is known please add a link to the issue in the pr before merging it. Just merging without comments in case the build fails feels like a bad practice

@merks
Copy link
Contributor

merks commented Dec 16, 2024

I’ve restarted so builds repeatedly until they pass lately. It think it causes global warming. Always the api baseline disposed. I think everyone has noticed this. So when the change is trivial and not os-specific I find other ways to spend my time.

@vogella
Copy link
Contributor

vogella commented Dec 16, 2024

You position is understandable. But it would be nice if PMC members and PLs demonstrate best practice and a quick "known build failure covered in #2603" might help the other team members to understand that we should not ignore build failures before merges.

It would also potentially help to raise the importance of #2603 to other committers if they she such issues referred too frequently

@merks
Copy link
Contributor

merks commented Dec 16, 2024

I've spent the entire day on builds across 5 projects (as well as the better part of my weekend) ensuring that lucene 10 rolls out through SimRel without a problem. So I'm slightly short tempered on getting advice about builds and how to manage them according to best practices. Your advice once is sufficient, your advice more than once is really more than enough.

At the end of such a day, I look at an issue like this one and I think, who's going to merge this if not me? Probably no one. Should I hit some buttons to make the builds all green like I did twice this weekend for the lucene 10 upgrades? I'm not entirely convinced that making more noise about a problem will compel someone else to solve that problem. I'll leave making noise to others and focus on getting things done quietly.

@vogella
Copy link
Contributor

vogella commented Dec 16, 2024

I only asked to put in a short link to the known issue or a short statement like "build failure looks unrelated".

I never suggested anything else @merks

IIRC the PMC in my time educated committers about that desired process so I assumed PMC members should also follow this process.

If the new process is that we silently merge if we think the verification build failure is already known or not relevant that new process should also be communicated.

At least a see other PMC members still putting in a note like "build failure seems unrelated".

@merks
Copy link
Contributor

merks commented Dec 16, 2024

Three times advice. Yes I’m a horrible example and I need to be told repeatedly because I just didn’t understand the first two times. Stupid, ignorant, or just lacking in social skills. Perhaps all three. My lack of social skills compels me to point out that there was a build failure that you noticed. You decided to leave it to someone else to investigate. Then when you noticed this wasn’t done you decided it necessary to point out three times that this is a bad practice. I would have hoped that after being exasperated over the weekend with this same problem you might cut me some slack. Hope dies last.

@vogella
Copy link
Contributor

vogella commented Dec 16, 2024

IMHO we might want to disable the API check on Linux if this process continues to fail on Linux. But that would be a discussion for #2603

@vogella
Copy link
Contributor

vogella commented Dec 16, 2024

I don't think anything negative of what you said about you @merks. I understand that it is super annoying to hear "hey please follow the process" after you spend an immense amount of time to solve issues.

Sounds like we both are stubborn and trying to bring our point across and that we both feel misunderstood.

@merks
Copy link
Contributor

merks commented Dec 16, 2024

Yes I’m very stubborn and super annoyed. I have no point to make.

Why we need to check api in 4 different builds that all run in parallel on the same content is a much more interesting question. It’s not just Linux that fails. It’s exasperating. I’ve lost patience with it. That’s bad of me. I don’t claim to be virtuous.

@vogella
Copy link
Contributor

vogella commented Dec 16, 2024

Why we need to check api in 4 different builds that all run in parallel on the same content is a much more interesting question.

I also fail to understand the usefulness of this. I guess the idea was to make API tools check stable on all platforms. Not sure if that check good be a separate check only running once.

@laeubi
Copy link
Contributor

laeubi commented Dec 17, 2024

Why we need to check api in 4 different builds that all run in parallel on the same content is a much more interesting question.

Because we have 3 different platform (especially on SWT where the source code can differ!) we need to support and we can't force people using a particular one. So we should make sure the build works "everywhere".

Another reason is diversity, if the check only fails on 1 out of 4 (same with tests) we can get some clue about

  1. Potential infra issue (e.g. it only fails on Jenkins but not on Github or vice versa)
  2. Potential instability (e.g. it fails once on windows once on linux once on mac)
  3. Potential platform dependent (e.g. always fails on mac)
  4. Of course we can have combinations of 1-3

Not sure if that check good be a separate check only running once.

The problem is that there are many "ideas" but only a few people took actually actions. e.g here is the list of API tools issues standing for years now an no one bother to fix them. I once even asked if we should drop api tools because no one seem to like maintain them and go instantly complains that these are super useful and we should improve:

I even have implemented that API tools only run if the bundle has changed, but the people complain they get "unrelated" warnings that has nothing to do with their code, so it is now reverted to check all bundles again and again. SO I'm really not eager to go that path again for "check API only once" with a lot of efforts for little gain.

Because another part of the truth is: If it works no one notice if it fails (even once) all start complaining and yelling. And even though there are problems here and there most of the time the checks just work, so its like with the red traffic lights, you only remember the one red but not the other 10 green light on your way.

azoitl added a commit to azoitl/eclipse.platform.releng.aggregator that referenced this pull request Dec 17, 2024
The eclipse.appName sytem property allows users to specify on the
command line the appName to be used for a specific Eclipse instance. The
appName Display property is used by window managers and
desktop environments for identification and grouping. This is helpful to
distinguish Eclipse instances (e.g., JDT vs CDT) as per default these
all are using the same identifier.

Eclipse UI PR:
eclipse-platform/eclipse.platform.ui#2631
Root Issue: eclipse-packaging/packages#253
azoitl added a commit to azoitl/www.eclipse.org-eclipse that referenced this pull request Dec 17, 2024
@azoitl
Copy link
Contributor Author

azoitl commented Dec 17, 2024

Added new and noteworthy entry: eclipse-platform/www.eclipse.org-eclipse#260

vogella pushed a commit to eclipse-platform/www.eclipse.org-eclipse that referenced this pull request Dec 18, 2024
akurtakov pushed a commit to azoitl/eclipse.platform.releng.aggregator that referenced this pull request Mar 3, 2025
The eclipse.appName sytem property allows users to specify on the
command line the appName to be used for a specific Eclipse instance. The
appName Display property is used by window managers and
desktop environments for identification and grouping. This is helpful to
distinguish Eclipse instances (e.g., JDT vs CDT) as per default these
all are using the same identifier.

Eclipse UI PR:
eclipse-platform/eclipse.platform.ui#2631
Root Issue: eclipse-packaging/packages#253
akurtakov pushed a commit to azoitl/eclipse.platform.releng.aggregator that referenced this pull request Mar 6, 2025
The eclipse.appName sytem property allows users to specify on the
command line the appName to be used for a specific Eclipse instance. The
appName Display property is used by window managers and
desktop environments for identification and grouping. This is helpful to
distinguish Eclipse instances (e.g., JDT vs CDT) as per default these
all are using the same identifier.

Eclipse UI PR:
eclipse-platform/eclipse.platform.ui#2631
Root Issue: eclipse-packaging/packages#253
akurtakov pushed a commit to azoitl/eclipse.platform.releng.aggregator that referenced this pull request Mar 7, 2025
The eclipse.appName sytem property allows users to specify on the
command line the appName to be used for a specific Eclipse instance. The
appName Display property is used by window managers and
desktop environments for identification and grouping. This is helpful to
distinguish Eclipse instances (e.g., JDT vs CDT) as per default these
all are using the same identifier.

Eclipse UI PR:
eclipse-platform/eclipse.platform.ui#2631
Root Issue: eclipse-packaging/packages#253
akurtakov pushed a commit to azoitl/eclipse.platform.releng.aggregator that referenced this pull request Mar 11, 2025
The eclipse.appName sytem property allows users to specify on the
command line the appName to be used for a specific Eclipse instance. The
appName Display property is used by window managers and
desktop environments for identification and grouping. This is helpful to
distinguish Eclipse instances (e.g., JDT vs CDT) as per default these
all are using the same identifier.

Eclipse UI PR:
eclipse-platform/eclipse.platform.ui#2631
Root Issue: eclipse-packaging/packages#253
akurtakov pushed a commit to azoitl/eclipse.platform.releng.aggregator that referenced this pull request Mar 18, 2025
The eclipse.appName sytem property allows users to specify on the
command line the appName to be used for a specific Eclipse instance. The
appName Display property is used by window managers and
desktop environments for identification and grouping. This is helpful to
distinguish Eclipse instances (e.g., JDT vs CDT) as per default these
all are using the same identifier.

Eclipse UI PR:
eclipse-platform/eclipse.platform.ui#2631
Root Issue: eclipse-packaging/packages#253
akurtakov pushed a commit to akurtakov/eclipse.platform.releng.aggregator that referenced this pull request Mar 18, 2025
The eclipse.appName sytem property allows users to specify on the
command line the appName to be used for a specific Eclipse instance. The
appName Display property is used by window managers and
desktop environments for identification and grouping. This is helpful to
distinguish Eclipse instances (e.g., JDT vs CDT) as per default these
all are using the same identifier.

Eclipse UI PR:
eclipse-platform/eclipse.platform.ui#2631
Root Issue: eclipse-packaging/packages#253
akurtakov pushed a commit to azoitl/eclipse.platform.releng.aggregator that referenced this pull request Mar 18, 2025
The eclipse.appName sytem property allows users to specify on the
command line the appName to be used for a specific Eclipse instance. The
appName Display property is used by window managers and
desktop environments for identification and grouping. This is helpful to
distinguish Eclipse instances (e.g., JDT vs CDT) as per default these
all are using the same identifier.

Eclipse UI PR:
eclipse-platform/eclipse.platform.ui#2631
Root Issue: eclipse-packaging/packages#253
akurtakov pushed a commit to eclipse-platform/eclipse.platform.releng.aggregator that referenced this pull request Mar 18, 2025
The eclipse.appName sytem property allows users to specify on the
command line the appName to be used for a specific Eclipse instance. The
appName Display property is used by window managers and
desktop environments for identification and grouping. This is helpful to
distinguish Eclipse instances (e.g., JDT vs CDT) as per default these
all are using the same identifier.

Eclipse UI PR:
eclipse-platform/eclipse.platform.ui#2631
Root Issue: eclipse-packaging/packages#253
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants